Skip to content

fix(docs): extract code snippets into dedicated python files and fix examples #1065

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 66 commits into from

Conversation

michaelbrewer
Copy link
Contributor

@michaelbrewer michaelbrewer commented Mar 7, 2022


TIPS FOR REVIEW:
Compare the PR docs (v1.25.7 + latest commit on this PR): https://gyft.github.io/aws-lambda-powertools-python/latest/
Against the current: https://awslabs.github.io/aws-lambda-powertools-python/latest/
See Doc Fixes section for notes on specific fixes
Run make format-examples lint-examples to format and lint extracted examples


Issue #, if available:

Description of changes:

Move code snippets into dedicated python files so they can be formatted (isort, black), and then embedded in the docs.

As part of the extraction a number of bugs was fixed, things like missing imports, python syntax errors, yaml formatting, invalid terraform and correct the line highlights.

A couple Makefile tasks where added to help verify changes (format-examplesmake task to runisortandblackon examples andlint-examplesmike task to runpython compileandcfn-linton examples).cfn-lint` may then become a dev dependency.

Directory Structure: Currently examples are moved under docs/examples and relates to the directory and file names of the *.md files: eg docs/core/logger.md examples are in docs/examples/core/logger/ folder.

Other things we can do: python syntax verification (black actually does a pretty good of checking for valid python) and cloudformation linting (cfn-lint) and sam validate.

Changes:

  • Add format-examples make task to run isort and black on examples
  • Add lint-examples make task to run python compile and cfn-lint on examples
  • Converted docs/index.md examples
  • Converted docs/core/event_handler/appsync.md examples
  • Converted docs/core/event_handler/api_gateway.md examples
  • Converted docs/core/logger.md examples
  • Converted docs/core/metrics.md examples
  • Converted docs/core/tracer.md examples
  • Converted docs/tutorial/index.md examples
  • Converted docs/utilities/batch.md examples
  • Converted docs/utilities/data_classes.md examples
  • Converted docs/utilities/feature_flags.md examples
  • Converted docs/utilities/idempotency.md examples
  • Converted docs/utilities/jmespath_functions.md examples
  • Converted docs/utilities/middleware_factory.md examples
  • Converted docs/utilities/parser.md examples
  • Converted docs/utilities/parameters.md examples
  • Converted docs/utilities/typing.md examples
  • Converted docs/utilities/validation.md examples

Doc Fixes:

NOTE: During the extraction the following fixes was made, which could be fix earlier in a separate PR

Questions on the docs

During the conversion process some questions have some up for later review

Other docs to extract

Only Doc string are remaining, which is also part of the generated api reference and may be resolved by #517

Checklist


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Mar 7, 2022
@pull-request-size pull-request-size bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Mar 7, 2022
@michaelbrewer michaelbrewer changed the title doc(data-classes): code snippets into dedicated python files docs(data-classes): code snippets into dedicated python files Mar 7, 2022
@pull-request-size pull-request-size bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 8, 2022
@codecov-commenter
Copy link

codecov-commenter commented Mar 8, 2022

Codecov Report

Merging #1065 (44f01ba) into develop (b577366) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #1065   +/-   ##
========================================
  Coverage    99.96%   99.96%           
========================================
  Files          119      119           
  Lines         5378     5378           
  Branches       613      613           
========================================
  Hits          5376     5376           
  Partials         2        2           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b577366...44f01ba. Read the comment docs.

@michaelbrewer
Copy link
Contributor Author

@michaelbrewer @heitorlessa A really important PR.

I think there's an issue with having these examples without giving counter-examples with the same code being used with the Parser. If the official examples have mostly data classes, why would anyone use the parser? they might not even notice it exists.

I am just working through was was already there. And there is a lot to go through still.

@michaelbrewer michaelbrewer changed the title docs(data-classes): code snippets into dedicated python files docs: code snippets into dedicated python files Mar 8, 2022
@michaelbrewer
Copy link
Contributor Author

@ran-isenberg to be clear, i am just looking at all of the existing code examples in the docs (and per the roadmap issue) to be extracted as snippets.

@ran-isenberg
Copy link
Contributor

ran-isenberg commented Mar 8, 2022

got it. Thanks for the quick reply. A misunderstanding on my part.

Michael Brewer added 2 commits March 9, 2022 09:44
Changes:
- move examoples into "docs/" so that mkdoc automatically picks up changes
- add a "format-examples" task to Makefile
@boring-cyborg boring-cyborg bot added the internal Maintenance changes label Mar 9, 2022
@github-actions github-actions bot added the bug Something isn't working label Apr 5, 2022
@michaelbrewer
Copy link
Contributor Author

@heitorlessa this is read to review. I have deployed a sync up version of the docs on gyft.github.io/aws-lambda-powertools-python/latest. Just an initial review would be helpful to see if this meets the requirements of the papercuts #1009

@jplock
Copy link

jplock commented Apr 9, 2022

I noticed in the docs we recommend using the Lambda layer, but that doesn’t work with code signing. Should we 1/ sign the layer so the signer could be shared or 2/ mention that code signing isn’t supported?

@michaelbrewer
Copy link
Contributor Author

I noticed in the docs we recommend using the Lambda layer, but that doesn’t work with code signing. Should we 1/ sign the layer so the signer could be shared or 2/ mention that code signing isn’t supported?

True that is a gap in the documentation. And considering this is a best practice, we should call it out.

@jplock
Copy link

jplock commented Apr 10, 2022

I don't know how difficult it would be to sign the layer and share the signer in the same account that is hosting the layer, but I'd like to be able to use the layer with code signing myself. :)

@heitorlessa
Copy link
Contributor

Hi @michaelbrewer, I appreciate your bias for action but this is unrealistic to review (66730K LOC changes), especially under our limited bandwidth.

Could you please break this down into smaller PRs - one per utility - and discard Event Source Data Classes?

Event Source Data Classes doc will be overhauled entirely. We will be looking into auto-generating that doc to better assist customers looking for a snippet as well as properties and methods.

Thank you

Copy link
Contributor

@heitorlessa heitorlessa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please break this down into smaller PRs - one per utility - and discard Event Source Data Classes changes?

@heitorlessa
Copy link
Contributor

heitorlessa commented Apr 12, 2022

I don't know how difficult it would be to sign the layer and share the signer in the same account that is hosting the layer, but I'd like to be able to use the layer with code signing myself. :)

@jplock

Created an issue - could you please +1 so we can look into it the future? Lambda Layers is the biggest piece of work we're currently planning right now, and we would need to test whether a non-signed function can use a signed Lambda Layer

#1108

@michaelbrewer
Copy link
Contributor Author

michaelbrewer commented Apr 12, 2022

Could you please break this down into smaller PRs - one per utility - and discard Event Source Data Classes changes?

discard Event Source Data Classes changes? what does this mean? I don't think the Event Sources Data Classes doc examples have any bugs, but it would benefit from the auto formatting and linting.

@michaelbrewer
Copy link
Contributor Author

michaelbrewer commented Apr 12, 2022

@heitorlessa can you please remove the triage from #1064 , so i can freely split this 8 day long PR into smaller ones to help fix the documentation errors.

Michael Brewer added 3 commits April 12, 2022 15:55
Changes:
- Update make tasks to include shared code examples
- Add missing dubug mode example
- Correct python hl_lines spacing
- Fix line highlights for validation and jmepath examples
- Move schema.py into examples for Validation
@heitorlessa heitorlessa deleted the docs/1064 branch April 13, 2022 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation internal Maintenance changes size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docs: Event Handler - Broken AWS Serverless Application Model (SAM) example
5 participants